Identity-async safety: thread-scoped HoistIdentity cache + propagation#564
Draft
lbwexler wants to merge 3 commits into
Draft
Identity-async safety: thread-scoped HoistIdentity cache + propagation#564lbwexler wants to merge 3 commits into
lbwexler wants to merge 3 commits into
Conversation
…1 plan doc Completes the new-file portion of the identity/async-safety changes already committed in 4383aa4. See docs/planning/identity-async-safety.md. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…, consolidations - Single install method `installThreadIdentity(HoistIdentity)` (null = clear) plus typed installers `installIdentityFromRequest` and `installIdentityFromWebSocketSession`. - HoistWebSocketHandler now installs identity around each lifecycle callback, so message handlers and channel construction can use the standard `identityService` accessors. - Dropped lazy session-read from accessors — they read the thread cache directly. Removed `findAuthUser`, `getSessionIfExists`, `findHoistUser`, `cleanupThreadIdentity`, and the now-redundant legacy `threadUsername`/`threadAuthUsername` plumbing. - Consolidated mutators around private `setIdentity` / `clearIdentity` helpers so session writes happen in exactly one place. - Made `AUTH_USER_KEY` / `APPARENT_USER_KEY` private; `HoistWebSocketChannel` reaches identity through the service rather than session-attribute key constants. - CHANGELOG entry added. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Phase 1 of docs/planning/identity-async-safety.md. Makes identity safe to read on threads that outlive the originating HTTP request (async continuations, auto-instrumented spans, cluster tasks) by moving the source of truth from the live request/session to a thread-scoped
HoistIdentitycache, populated lazily on request threads and propagated explicitly across async boundaries.Fixes the
IllegalStateException: The request object has been recycledthrown byTraceService.createSpan→IdentityService.getAuthUsername→request.getSession()(and the same shape viaTagSpanProcessor.onStart,TrackService.parseSubmittedEntry).Changes
HoistIdentity(immutable POGO:username,authUsername)IdentityPropagatingPromiseFactory— installed at startup, propagates identity into Grailstask {}workersIdentityServicerefactored to a singleThreadLocal<HoistIdentity>; accessors read cache; mutators update cache + session in lock-step;getSessionIfExistscatchesIllegalStateExceptionfrom recycled facadesHoistFilterclears the cache infinallyto prevent leakage on pooled threadsClusterTaskinstalls/clears via the unified cacheTrackService/browser.Utilsuse asafeHeaderhelper so observability reads tolerate recycled facadesOut of scope
Phase 2 (eliminate live-request propagation into workers via a
runDetachedsibling torunAsync) is optional and not included here. See the plan doc.Test plan
./gradlew assemble— confirmed locally)IllegalStateException🤖 Generated with Claude Code